-
Notifications
You must be signed in to change notification settings - Fork 134
*: lock supporting grace period #4226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4226 +/- ##
==========================================
+ Coverage 56.96% 57.01% +0.05%
==========================================
Files 237 237
Lines 30688 30721 +33
==========================================
+ Hits 17482 17517 +35
+ Misses 10976 10974 -2
Partials 2230 2230 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
| // New returns new private key locking service. It errors if a recently-updated private key lock file exists. | ||
| func New(path, command string) (Service, error) { | ||
| content, err := os.ReadFile(path) | ||
| func New(privKeyFilePath, clusterLockFilePath, command string) (Service, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: can we rename that to privKeyFileLockPath? Reading it as privKeyFilePath I've assumed it's the actual privKey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used to follow "filepath" convention as a common widely adopted term. For instance, the standard Golang's package "path/filepath" is an example.
| elapsedPeriod := time.Since(meta.Timestamp) | ||
| if elapsedPeriod < gracePeriod { | ||
| waitTime := gracePeriod - elapsedPeriod | ||
| errText := fmt.Sprintf("existing private key lock file found with different cluster lock hash, you must wait for %v before starting charon with the new cluster hash", waitTime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| errText := fmt.Sprintf("existing private key lock file found with different cluster lock hash, you must wait for %v before starting charon with the new cluster hash", waitTime) | |
| errText := fmt.Sprintf("An existing private key lock file is present with a different cluster lock hash, for safety reasons, you must wait for %v before starting charon with a modified cluster", waitTime) |
Idk will



Add cluster lock hash validation to privkey lock file
Enhances the private key lock mechanism to prevent race conditions during cluster configuration changes.
Changes
cluster_lock_hashin the privkey lock file metadataprivkeylock.New()signature to accept cluster lock file pathWhy
After cluster edits (adding/removing operators), validators shouldn't immediately start with the new configuration. The grace period ensures:
category: feature
ticket: #4200